-
Notifications
You must be signed in to change notification settings - Fork 141
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reorder paths: path reversal is now possible #91
base: master
Are you sure you want to change the base?
Conversation
…path commands like A are not supported), it now handles groups by considering them as a single path so it will not modify the order inside of the group and at the moment, it cannot reverse a group
inkscape_driver/eggbot_reorder.inx
Outdated
your document before running this routine. | ||
</_param> | ||
</_param> | ||
<param name="allowReverse" type="boolean" _gui-text="Allow Reverse">true</param> | ||
|
||
<!-- | ||
<param name="reverse" type="boolean" _gui-text="Allow paths to be reversed*">false</param> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove the "todo" item for a reverse param in this comment block (line 32).
inkscape_driver/eggbot_reorder.py
Outdated
these comparison distances into something more relevant such as degrees traveled? | ||
Uses a greedy algorithm which can reverse the path direction if necessary | ||
Returns a list of (id, reverse), as well as the original and optimized | ||
"air distance" which is just the distance traveled in the air. Reverse indicate if the path must be reversed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A param name of "allowReverse" seems to conflict with the description here of "must be reversed". How does this work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If allowReverse==False
, all the reverse
booleans will be False.
Thank you for this contribution, Daekkyn. I've been working on a separate set of improvements to this extension (diving into groups and so forth), and I'm a little conflicted about merging this for a few reasons. (1) The big idea implemented (reversing paths in the TSP) is unambiguously a step forward. To the extent that this extension is intended to produce output that can be used on the EggBot, AxiDraw, or other tools that may be expecting it, one could consider the following approach: (A) Have the reorder extension "label" each path that should be reversed, perhaps with an attribute like The potential advantages of this approach are:
The big disadvantage of this approach is:
I'm open to discussion on it. :) |
I have already tested on many SVGs and it worked, but more testing is always good. I guess adding a label is also an option, but I'm not a big fan of adding a non SVG-standard elements if there is another solution. I agree that it's easier to implement and test. However the speed improvement would be negligible, reversing the paths has a linear complexity and takes milliseconds on practice, even on big files. |
I was suggesting an attribute on the SVG path tag, not a different element. ( Per the SVG specification, this kind of thing explicitly allowed: "SVG allows inclusion of attributes from foreign namespaces on any SVG element. " https://www.w3.org/TR/SVG/extend.html ). In any case, let's discuss the code a little bit. Looking a bit deeper, I'm concerned that there are quite a few changes in the code that aren't just about path reversal. This makes it much harder to integrate (or even compare) with the other changes that I've been working on.
|
|
If there is more things to change, feel free to edit the code. |
# Conflicts: # inkscape_driver/eggbot_reorder.py
Improves the greedy algorithm by allowing paths to be reversed (some path commands like A are not supported).
Also, it now handles groups by considering them as a single path so it will not modify the order inside of the group. At the moment, it cannot reverse a group.